Thank you for your contribution to sbpy, an Astropy affiliated package! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

• Do the proposed changes actually accomplish desired goals?
• Do the proposed changes follow the sbpy coding guidelines?
• Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
• Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
• Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
• Did the CI pass? If no, are the failures related?
• Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label.
• Is a milestone added?

Keeping this a draft until #427 is done.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.09%. Comparing base (59a8a49) to head (408c258).

@@            Coverage Diff             @@
##             main     #434      +/-   ##
==========================================
+ Coverage   81.81%   82.09%   +0.28%     
==========================================
  Files          53       53              
  Lines        4267     4335      +68     
==========================================
+ Hits         3491     3559      +68     
  Misses        776      776              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Updated comment with API-breaking change in to_ephem.

I didn't see any notable issues in the code changes and the functionality seems good after the various API and other changes to the previous way of doing things. My main comments are on the documentation:

• From the documentation, I was confused for a while about the relationship between SourceOrbit and source_orbit, and which the user was meant to use. The SourceOrbit class is mentioned but there is no example of its use in the documentation, while source_orbit (from SynGenerator) is the method used in the example to plot the object's orbit, so the need for a new SourceOrbit class was initially unclear. The change log makes it clearer that the source_orbit method produces a SourceOrbit object (so the SourceOrbit class mainly just enables backend functionality but is never directly called by name by the user?), but maybe this distinction should be made in the documentation itself? At the moment, in the documentation, under "Plotting syndynes and synchrones", it should probably say: "Syndynes, synchrones, and source_orbit have plot() methods to assist with this." since the example shows source_orbit.plot(), not SourceOrbit.plot().

• I'm not sure if this is a system-specific thing, but the synchrones in the syndyne-synchrone plot example in the documentation do not show up as dashed lines for me as in the example plot (shows up as all solid lines for me), so that plotting call should be "dust.synchrones()[4::5].plot(ax,ls="--")"

• It would probably be useful in the documentation to move the example where different syndynes are plotted with different line styles above the Spitzer example, because the variables get set to different values in the Spitzer example, whereas the sample output plot showing different line styles uses syndyne data from before the Spitzer example. Also, in that last example (plotting different line styles), while it probably shouldn't be too difficult for a user to figure out, it might still be better to write out the full code for generating the sample plot, rather than just showing the lines that are different from the original example code and leaving it to the user to fill in the blanks.

• At one point, "telescope" is incorrectly spelled as the "Spitzer Space Telesocpe"

From the documentation, I was confused for a while about the relationship between SourceOrbit and source_orbit, and which the user was meant to use. The SourceOrbit class is mentioned but there is no example of its use in the documentation, while source_orbit (from SynGenerator) is the method used in the example to plot the object's orbit, so the need for a new SourceOrbit class was initially unclear. The change log makes it clearer that the source_orbit method produces a SourceOrbit object (so the SourceOrbit class mainly just enables backend functionality but is never directly called by name by the user?), but maybe this distinction should be made in the documentation itself?

Yeah, I see what you are saying. I'll move the orbit section to be contiguous with the syndyne and synchrone sections, and add some words before all this to clarify what is happening here: the SynGenerator object has methods syndynes(), synchrones(), and source_orbit() which produce Syndynes, Synchrones, and SourceOrbit objects.

At the moment, in the documentation, under "Plotting syndynes and synchrones", it should probably say: "Syndynes, synchrones, and source_orbit have plot() methods to assist with this." since the example shows source_orbit.plot(), not SourceOrbit.plot().

I think the above edits should help explain why this doesn't work. source_orbit is a function that produces the SourceOrbit object. source_orbit itself does not have a plot method. I think my attempt to be brief has confused things a bit, so I'll expand the example to help.

I'm not sure if this is a system-specific thing, but the synchrones in the syndyne-synchrone plot example in the documentation do not show up as dashed lines for me as in the example plot (shows up as all solid lines for me), so that plotting call should be "dust.synchrones()[4::5].plot(ax,ls="--")"

fixed

It would probably be useful in the documentation to move the example where different syndynes are plotted with different line styles above the Spitzer example, because the variables get set to different values in the Spitzer example, whereas the sample output plot showing different line styles uses syndyne data from before the Spitzer example.

That makes sense. Moved!

Also, in that last example (plotting different line styles), while it probably shouldn't be too difficult for a user to figure out, it might still be better to write out the full code for generating the sample plot, rather than just showing the lines that are different from the original example code and leaving it to the user to fill in the blanks.

Instead of repeating everything, I have enable the full "source code" link for the figure. Is that OK?

At one point, "telescope" is incorrectly spelled as the "Spitzer Space Telesocpe"

And, done.

@hhsieh00 ready for your reconsideration